Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

refactor(minErr): code cleanup #10601

Closed
wants to merge 1 commit into from

Conversation

LeonardoBraga
Copy link
Contributor

Removes a "magic number" (2) used multiple times in the code.
Removes unnecessary variables "arg" and "prefix".
Removed a condition within the "for" loop that generates query string parameters.
Adds a test case to ensure URL parameters are generated as expected.

Removes a "magic number" (2) used multiple times in the code.
Removes unnecessary variables "arg" and "prefix".
Removed a condition within the "for" loop that generates query string parameters.
Adds a test case to ensure URL parameteres are generated as expected.
@ilanbiala
Copy link

@LeonardoBraga can you show an example of the improvement? It seems like it's more just cleanup..

@LeonardoBraga
Copy link
Contributor Author

@ilanbiala You're correct, one can say it's more of a cleanup that removes a few things and changes others. The changes I mentioned in the comment are the (minor) improvements I referred to, plus the test case for the error URL parameters.

This is my first PR, so please accept my apologies if the term "improvement" wasn't a good fit for the description of these changes.

@ilanbiala
Copy link

@LeonardoBraga it's fine, you might just want to edit the PR title for posterity.

@LeonardoBraga LeonardoBraga changed the title refactor(minErr): better minification and other improvements refactor(minErr): code cleanup Dec 30, 2014
@LeonardoBraga
Copy link
Contributor Author

@ilanbiala Done. Please let me know if you have another suggestion for it. Thanks.

@LeonardoBraga
Copy link
Contributor Author

After applying the changes requested by @ilanbiala, I would like to know if there are any other adjustments or questions that might be holding this PR. Thanks!

@lgalfaso
Copy link
Contributor

lgalfaso commented Jan 6, 2015

@LeonardoBraga will review this in the next day or two

@lgalfaso lgalfaso self-assigned this Jan 6, 2015
@LeonardoBraga
Copy link
Contributor Author

Thanks, @lgalfaso!

@LeonardoBraga
Copy link
Contributor Author

@lgalfaso did you have the opportunity to take a look at this one?

@lgalfaso
Copy link
Contributor

landed as b146cae

@lgalfaso lgalfaso closed this Jan 10, 2015
@LeonardoBraga LeonardoBraga deleted the refactor-minerr branch January 10, 2015 20:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants